Skip to content

Conversation

ayushb
Copy link
Member

@ayushb ayushb commented Sep 15, 2026

Solves #6 and #12.

This is #23, #24 and #25 rolled into one branch so it can go to main in a single review. Commit history is kept, so the four commits can still be read separately.

main has not moved since issue-6 branched, so this merges cleanly.

What is in it

1. chore: glue components together and fix: delete file that was added by accident (@thomhet, originally #23)

Combines the components in App, centralises variables and util functions, adds the responsive layout, colours the type badges.

2. fix(mvp): repair build errors and fetch through TanStack Query (originally #24)

npm run build did not complete on issue-6: 14 TypeScript errors and 6 lint errors. The tests passed, which is why it read as green. Fixed here:

  • SORT_OPTIONS had been removed while isSortOption still called it, and sort was written with an unchecked cast, so a corrupt value in session storage passed straight through. Validation restored.
  • GetMany pushed failed fetches into a PokemonData[] as null, which the favorites view then crashed on. It fetches in parallel now and drops failures.
  • matchesFilter took one argument but was called with two in GetPrevFiltered and GetNextFiltered.
  • FavoritePokemon called .sort() and onSelectPokemon() on props typed as optional, and sorted the caller's array in place.
  • Untyped index access in PokemonCard and filters.ts, and total reassigned during render.

Behaviour, on top of the build:

  • Navigating to a new pokemon kept showing the previous one's sprite, because the sprite lived in useState and the card never remounts. Confirmed with a render test, fixed by resetting on id change.
  • PokemonList and useFavorites fetched with plain useState and useEffect. Both go through TanStack Query now, which is a requirement and also what was causing the repeated refetching.
  • The favorites entries and the sprite toggle were div and li with onClick, so neither could be reached by keyboard. Both are buttons with focus outlines.
  • App rendered a second id="root" inside the one in index.html. Ids have to be unique, so the layout div is .app-layout.

3. feat(filters): add filter component that reloads the pokemon list (originally #25)

The filter view from #12. Sort by id, filter on any number of types, and only-show-favorites. Collapsed behind a details / summary so eighteen type checkboxes do not crowd the header row, which also gives keyboard and screen reader support without a third party component. Rules persist in session storage through useFilters.

sort and onlyFavorites were in the model but read nowhere, so they did nothing. Both are applied now. The favorites view reuses what useFavorites already fetched rather than calling the api again.

Sorting is limited to id-asc and id-desc. Sorting on name would mean fetching every pokemon up front just to learn the names, which the fetch-on-the-fly rule does not allow.

Checks

on issue-6 here
tsc -b 14 errors clean
eslint . 6 errors clean
prettier --check 1 file clean
vitest run 34 passed, 7 files 48 passed, 9 files
vite build fails builds

PokemonList.test.tsx had been deleted in #23 and is restored here.

Known, not fixed in this PR

GetNextFiltered and GetPrevFiltered walk one id at a time, fetching every pokemon until one matches. With a sparse type filter, filling an 11 item list is a lot of sequential requests, and a filter that matches nothing scans all 1025 in both directions. That is the "no unnecessary calls to the REST API" criterion. PokeAPI's /type/{name} returns every pokemon of a type in one request and looks like the right fix, but it is a large enough change that it belongs in its own issue.

Still open elsewhere: #10 tests (FavoritePokemon and api/pokemon.ts have none), #11 css polish, #13 deploy, #14 readme.

Thomas-Andre Hetlesaether and others added 4 commits September 14, 2026 10:59
- combine components in app.tsx
- centralize variables and util functions
- add layout for responsible design
- add color to type badges
- improve css
- ready for milestone: MVP
- lint and test cases pass
* Restore SORT_OPTIONS and validate sort on load instead of casting it
* Limit SortOption to id-asc and id-desc, name sorting needs a full preload
* Pass filter rules into GetPrevFiltered and GetNextFiltered instead of
  reading session storage once per pokemon
* Fetch favorites in parallel and drop the ids that failed
* Reset PokemonCard to the default sprite when a new pokemon is shown
* Move PokemonList and useFavorites onto TanStack Query
* Render favorite entries and the sprite toggle as buttons for keyboard access
* Replace the duplicate id="root" wrapper with .app-layout
* Add a shared QueryClient wrapper for hook and component tests

References #6
* Build Filter as a presentational component with filters, onChange and onReset props
* Collapse the panel behind a details summary that counts the active rules
* Let the user pick sort order, any number of types and favorites only
* Apply the sort order and the favorites rule when building the list
* Reuse the favorites already fetched instead of asking the api again
* Drop the unused FilterSettings type left over from the filter model
* Register the jest-dom matcher types so tests can assert on checked state
* Add tests for the component and for the list it filters

References #12
…tion

* Add api tests that stub fetch so nothing reaches the network
* Add FavoritePokemon tests for props, ordering, selection and the empty state
* Add PokemonCard tests for stepping through sprites and resetting on a new pokemon
* Add GetNextFiltered and GetPrevFiltered tests, including recovery from a failed fetch
* Add App tests that check navigation and that choices reach local and session storage
* Add snapshots for the favorites view

References #10
thomhet
thomhet previously approved these changes Sep 16, 2026
Copy link
Member

@thomhet thomhet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

test(coverage): cover the api, the favorites view and filtered navigation
Copy link
Member

@thomhet thomhet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ayushb ayushb merged commit 308d159 into main Sep 16, 2026
@thomhet thomhet deleted the feat/mvp-filters-and-build-fixes branch September 18, 2026 22:58
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants